Strip ascriptions originating from `PrepareInlineable.wrapRHS` or `Typer.ensureNoLocalRefs` during inlining #24425

mbovel · 2025-11-13T19:57:26Z

Attempt to fix #24420 by removing ascriptions during inlining when they originate from PrepareInlineable.wrapRHS or Typer.ensureNoLocalRefs.

sjrd · 2025-11-13T20:54:20Z

Isn't that basically making all inlined def into transparent?

tests/run/i24420-inline-local-ref.scala

mbovel · 2025-11-13T23:31:36Z

Isn't that basically making all inline def behave like transparent?

Maybe, and that is probably not what we want.

So when should the result of calling a non-transparent inline method actually be visible?

[Comment extracted to #24431]

mbovel · 2025-11-14T17:16:47Z

Isn't that basically making all inlined def into transparent?

As discussed orally, no, assuming we put the stripping logic at the right place: in the Inliner, to make sure that we only do it during inlining.

mbovel · 2025-11-15T12:36:56Z

Edit: temporary fixed by 6ae39ef and 8b803e1.

Dropping ascriptions has strange effects, making YCheckPositions fail, and probably more.

Consider tests/pos/i21320b.scala, shortened to:

import scala.deriving.*
import scala.compiletime.*

trait ConfigMonoid[T]:
  def zero: T
  def orElse(main: T, defaults: T): T

object ConfigMonoid:
  given option: [T] => ConfigMonoid[Option[T]] = ???

  inline def zeroTuple[C <: Tuple]: Tuple =
    inline erasedValue[C] match
      case _: EmptyTuple => EmptyTuple
      case _: (t *: ts) =>
        summonInline[ConfigMonoid[t]].zero *: zeroTuple[ts]

  inline def valueTuple[C <: Tuple, T](index: Int, main: T, defaults: T): Tuple =
    inline erasedValue[C] match
      case _: EmptyTuple => EmptyTuple
      case _: (t *: ts) =>
        def get(v: T) = v.asInstanceOf[Product].productElement(index).asInstanceOf[t]
        summonInline[ConfigMonoid[t]].orElse(get(main), get(defaults)) *: valueTuple[ts, T](
          index + 1,
          main,
          defaults
        )

  inline given derive: [T] => (m: Mirror.ProductOf[T]) => ConfigMonoid[T] =
    new ConfigMonoid[T]:
      def zero: T = m.fromProduct(zeroTuple[m.MirroredElemTypes])
      def orElse(main: T, defaults: T): T = m.fromProduct(valueTuple[m.MirroredElemTypes, T](0, main, defaults))

final case class PublishContextualOptions(
  v1: Option[String] = None,
  v2: Option[String] = None,
  v3: Option[String] = None,
)
object PublishContextualOptions:
  implicit val monoid: ConfigMonoid[PublishContextualOptions] = ConfigMonoid.derive // was crash

We get the following error:

     while compiling: tests/pos/i21320b.scala
        during phase: Ycheck
                mode: Mode(ImplicitsEnabled)
     library version: version 3.8.0-RC1-bin-SNAPSHOT-nonbootstrapped
    compiler version: version 3.8.0-RC1-bin-SNAPSHOT-nonbootstrapped-git-96b1b83
            settings: -Vprint List(inlining) -Ycheck List(all) -classpath /localhome/bovel/scala3/library/target/scala-library-nonbootstrapped/scala-library-3.8.0-RC1-bin-SNAPSHOT-nonbootstrapped.jar -d /localhome/bovel/scala3/out/default-last-scalac-out.jar
Exception in thread "main" java.lang.AssertionError: assertion failed: wrong source set for x$1 # -1 of class dotty.tools.dotc.ast.Trees$Ident, set to tests/pos/i21320b.scala but context had library/src/scala/Tuple.scala
 <synthetic> <touched>
        at scala.runtime.Scala3RunTime$.assertFailed(Scala3RunTime.scala:10)
        at dotty.tools.dotc.transform.YCheckPositions$$anon$1.traverse(YCheckPositions.scala:36)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1817)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1817)
        at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.fold$1(Trees.scala:1681)
        at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.apply(Trees.scala:1683)
        at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1700)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.traverseChildren(Trees.scala:1818)
        at dotty.tools.dotc.transform.YCheckPositions$$anon$1.traverse(YCheckPositions.scala:55)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1817)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1817)
        at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1694)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.traverseChildren(Trees.scala:1818)
        at dotty.tools.dotc.transform.YCheckPositions$$anon$1.traverse(YCheckPositions.scala:55)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1817)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1817)
        at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1702)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.traverseChildren(Trees.scala:1818)
        at dotty.tools.dotc.transform.YCheckPositions$$anon$1.traverse(YCheckPositions.scala:55)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1817)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.apply(Trees.scala:1817)
        at dotty.tools.dotc.ast.Trees$Instance$TreeAccumulator.foldOver(Trees.scala:1708)
        at dotty.tools.dotc.ast.Trees$Instance$TreeTraverser.traverseChildren(Trees.scala:1818)
        at dotty.tools.dotc.transform.YCheckPositions$$anon$1.traverse(YCheckPositions.scala:55)
...

The zero method for Monoid[PublishContextualOptions] looks like this on main:

def zero: PublishContextualOptions =
              m$proxy1.fromProduct(
                {
                  {
                    val x$1: Option[String] = ConfigMonoid.option[String].zero
                    {
                      val Tuple_this: Tuple =
                        {
                          {
                            val x$1: Option[String] =
                              ConfigMonoid.option[String].zero
                            {
                              val Tuple_this: Tuple =
                                {
                                  {
                                    val x$1: Option[String] =
                                      ConfigMonoid.option[String].zero
                                    {
                                      val Tuple_this: Tuple =
                                        {
                                          EmptyTuple
                                        }:Tuple
                                      runtime.Tuples.cons(x$1, Tuple_this).
                                        asInstanceOf[*:[Option[String], Tuple]]:
                                        *:[Option[String], Tuple]
                                    }
                                  }
                                }:Tuple
                              runtime.Tuples.cons(x$1, Tuple_this).asInstanceOf[
                                *:[Option[String], Tuple]]:
                                *:[Option[String], Tuple]
                            }
                          }
                        }:Tuple
                      runtime.Tuples.cons(x$1, Tuple_this).asInstanceOf[
                        *:[Option[String], Tuple]]:*:[Option[String], Tuple]
                    }
                  }
                }:Tuple

And with my changes:

def zero: PublishContextualOptions =
              m$proxy1.fromProduct(
                {
                  {
                    val x$1: Option[String] = ConfigMonoid.option[String].zero
                    {
                      val x$1: Option[String] = ConfigMonoid.option[String].zero
                      val x$1: Option[String] = ConfigMonoid.option[String].zero
                      val Tuple_this: EmptyTuple.type = EmptyTuple
                      val Tuple_this: Option[String] *: Tuple =
                        runtime.Tuples.cons(x$1, Tuple_this).asInstanceOf[
                          *:[Option[String], Tuple]]:*:[Option[String], Tuple]
                      {
                        val Tuple_this: Option[String] *: Tuple =
                          runtime.Tuples.cons(x$1, Tuple_this).asInstanceOf[
                            *:[Option[String], Tuple]]:*:[Option[String], Tuple]
                        runtime.Tuples.cons(x$1, Tuple_this).asInstanceOf[
                          *:[Option[String], Tuple]]:*:[Option[String], Tuple]
                      }
                    }
                  }
                }
              )

Which is incorrect.

My current guess is that dropping ascriptions enables eliding expressions, which in turns create other problems.

Need to further investigate…

Note: I tried to narrow the ascriptions stripping to only ascriptions created by PrepareInlinable.wrapRHS, but that doesn't solve the problem. I also tried to use a proper TreeMap to remove them, or to do it directly in Inliner.typedTyped.

mbovel · 2025-11-15T15:11:06Z

My current guess is that dropping ascriptions enables eliding expressions, which in turns create other problems.

Bingo, the culprit is liftBindings; if I disable it for blocks, then the tests pass again.

mbovel · 2025-11-16T10:33:00Z

Edit: fixed by 8b803e1. 6ae39ef was incomplete because it lost the capability to inline calls inside blocks, returning EmptyTrees instead.

The remaining failing test is tests/pos/i18123.scala. It fails because I disabled lifting blocks in liftBindings.

Minimal reproduction:

extension (x: String)
  transparent inline def rep(min: Int = 0): String = ???
def y: String = ???
def z = y.rep().toUpperCase

When lifting blocks, it fails (as expected?) with a type error:

-- [E007] Type Mismatch Error: tests/pos/i18123b.scala:6:8 ---------------------
6 |def z = y.rep().toUpperCase
  |        ^^^^^^^
  |Found:    (??? : => Nothing)
  |Required: ?{ toUpperCase: ? }
  |Note that implicit conversions were not tried because the result of an implicit conversion
  |must be more specific than ?{ toUpperCase: <?> }
  |
  | longer explanation available when compiling with `-explain`

But if we don't lift blocks, then we get:

-- Error: tests/pos/i18123b.scala:6:8 ------------------------------------------
6 |def z = y.rep().toUpperCase
  |        ^^^^^^^
  |        <notype> is illegal as a selection prefix

Which is due to the following adaptation:

adapt1(
  tree = {
    val x$1: String = y
    rep(x$1)(i18123b$package.rep$default$2(x$1))
  },
  pt = ?{ toUpperCase: ? }
) == <empty>

…lineTyper.typedTyped`

mbovel · 2025-11-16T15:25:25Z

I temporary solved the problems described in my 3 last comments by disabling lifting blocks in liftBindings. I suspect there is a problem when computing positions there. It's probably fixable, but I am not sure how.

Remaining, unrelated test failure:

[info] Test dotty.tools.dotc.semanticdb.SemanticdbTests.expectTests started
/home/runner/work/scala3/scala3/tests/semanticdb/javacp/com/javacp/Test.java:69: warning: [strictfp] as of release 17, all floating-point expressions are evaluated strictly and 'strictfp' is not required
    strictfp void strictfpMethod() {}
                  ^
1 warning
[error] check file /home/runner/work/scala3/scala3/tests/semanticdb/metac.expect does not match generated.
If you meant to make a change, replace the expect file by:
  mv /home/runner/work/scala3/scala3/tests/semanticdb/metac.expect.out /home/runner/work/scala3/scala3/tests/semanticdb/metac.expect
inspect with:
  diff /home/runner/work/scala3/scala3/tests/semanticdb/metac.expect /home/runner/work/scala3/scala3/tests/semanticdb/metac.expect.out
Or else update all expect files with
  sbt 'scala3-compiler-bootstrapped/Test/runMain dotty.tools.dotc.semanticdb.updateExpect'
Error:  Test dotty.tools.dotc.semanticdb.SemanticdbTests.expectTests failed: java.lang.AssertionError: 1 errors in expect test., took 1.729 sec
Error:      at dotty.tools.dotc.semanticdb.SemanticdbTests.runExpectTest(SemanticdbTests.scala:108)
Error:      at dotty.tools.dotc.semanticdb.SemanticdbTests.expectTests(SemanticdbTests.scala:65)
Error:      at jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
Error:      at jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
Error:      at jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
Error:      at java.lang.reflect.Method.invoke(Method.java:569)
Error:      ...

To be investigated.

mbovel mentioned this pull request Nov 13, 2025

inline val not inlined #24420

Open

som-snytt reviewed Nov 13, 2025

View reviewed changes

tests/run/i24420-inline-local-ref.scala Outdated Show resolved Hide resolved

mbovel mentioned this pull request Nov 14, 2025

Enhance constant-folding during inlining #24431

Open

mbovel force-pushed the mb/inline-strip-synthetic-ascriptions branch 2 times, most recently from 13d4c32 to ae88b64 Compare November 15, 2025 15:10

mbovel force-pushed the mb/inline-strip-synthetic-ascriptions branch from 8e5b86f to a8240f8 Compare November 15, 2025 15:52

mbovel added 6 commits November 16, 2025 13:24

Ignore ascriptions from ensureNoLocalRefs in ReTyper

202b196

Ignore ascriptions from PrepareInlinable.wrapRHS in InlineTyper

504cb78

Move stripping of ascriptions from PrepareInlineable.wrapRHS to `In…

ae33ea6

…lineTyper.typedTyped`

Do not lift bindings for blocks

6ae39ef

Only strip ascriptions in nested inline calls

3d68bba

Recurse through blocks in inlineCall

8b803e1

mbovel force-pushed the mb/inline-strip-synthetic-ascriptions branch from 82155ed to 8b803e1 Compare November 16, 2025 13:36

mbovel changed the title ~~Strip synthetic ascriptions from inlined calls~~ Strip ascriptions originating from PrepareInlineable.wrapRHS or Typer.ensureNoLocalRefs during inlining Nov 16, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Strip ascriptions originating from `PrepareInlineable.wrapRHS` or `Typer.ensureNoLocalRefs` during inlining #24425

Strip ascriptions originating from `PrepareInlineable.wrapRHS` or `Typer.ensureNoLocalRefs` during inlining #24425

mbovel commented Nov 13, 2025 •

edited

Loading

Uh oh!

sjrd commented Nov 13, 2025

Uh oh!

Uh oh!

mbovel commented Nov 13, 2025 •

edited

Loading

Uh oh!

mbovel commented Nov 14, 2025

Uh oh!

mbovel commented Nov 15, 2025 •

edited

Loading

Uh oh!

mbovel commented Nov 15, 2025

Uh oh!

mbovel commented Nov 16, 2025 •

edited

Loading

Uh oh!

mbovel commented Nov 16, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Strip ascriptions originating from PrepareInlineable.wrapRHS or Typer.ensureNoLocalRefs during inlining #24425

Are you sure you want to change the base?

Strip ascriptions originating from PrepareInlineable.wrapRHS or Typer.ensureNoLocalRefs during inlining #24425

Conversation

mbovel commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sjrd commented Nov 13, 2025

Uh oh!

Uh oh!

mbovel commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbovel commented Nov 14, 2025

Uh oh!

mbovel commented Nov 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbovel commented Nov 15, 2025

Uh oh!

mbovel commented Nov 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbovel commented Nov 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Strip ascriptions originating from `PrepareInlineable.wrapRHS` or `Typer.ensureNoLocalRefs` during inlining #24425

Strip ascriptions originating from `PrepareInlineable.wrapRHS` or `Typer.ensureNoLocalRefs` during inlining #24425

mbovel commented Nov 13, 2025 •

edited

Loading

mbovel commented Nov 13, 2025 •

edited

Loading

mbovel commented Nov 15, 2025 •

edited

Loading

mbovel commented Nov 16, 2025 •

edited

Loading

mbovel commented Nov 16, 2025 •

edited

Loading